Skip to content

feat: comprehensive MCP service tool improvements#30

Merged
cevian merged 9 commits intomainfrom
age-160-improve-service-tool-definitions
Oct 7, 2025
Merged

feat: comprehensive MCP service tool improvements#30
cevian merged 9 commits intomainfrom
age-160-improve-service-tool-definitions

Conversation

@cevian
Copy link
Contributor

@cevian cevian commented Sep 30, 2025

Summary

Comprehensive improvements to MCP service tools including renamed tools, optimized descriptions, improved parameter clarity, and extensive documentation.

Changes

Tool Naming Improvements

  • Removed redundant tiger_ prefix from all MCP tools (e.g., tiger_service_listservice_list)
  • Simplified tool names for better usability

Description Optimization

  • Streamlined tool descriptions for LLM token efficiency
  • Removed redundant information while maintaining clarity
  • Focused descriptions on essential functionality

Parameter Improvements

  • Renamed timeout to timeout_minutes in service_create for explicit unit clarity
  • Improved parameter documentation with better examples and constraints
  • Enhanced schema generation with proper type definitions

Documentation

  • Added comprehensive MCP feedback document (mcp_feedback.md)
  • Analyzed tool naming conventions, parameter documentation, and API design
  • Documented implementation status and future improvements

Commits Included

  • d60341f fix: rename MCP tools to remove redundant tiger_ prefix
  • 3d22a25 feat: implement comprehensive MCP tool improvements
  • c58b29c docs: finalize MCP feedback implementation status
  • 62e3543 docs: add MCP tool description optimization analysis
  • 880f2e8 docs: update MCP feedback with implementation status
  • 17f087c refactor: streamline MCP tool descriptions for LLM optimization
  • a28eba9 refactor: rename timeout to timeout_minutes in service_create MCP tool

Test Plan

  • Code compiles successfully
  • MCP tools function correctly with new names
  • Schema generation produces valid JSON schemas
  • Parameter documentation is clear and accurate

🤖 Generated with Claude Code

@linear
Copy link

linear bot commented Sep 30, 2025

AGE-160 Tiger CLI: Improve service tool definitions

I did not spend much time on the tool definitions in AGE-121. They should almost certainly be iterated on and improved. See PR comment.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements comprehensive improvements to MCP service tools including renamed tools, optimized descriptions, improved parameter clarity, and extensive documentation.

  • Removed redundant tiger_ prefix from all MCP tools for better usability
  • Streamlined tool descriptions for LLM token efficiency while maintaining clarity
  • Renamed timeout to timeout_minutes in service creation for explicit unit clarity

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
mcp_feedback.md Added comprehensive MCP feedback documentation analyzing tool improvements and implementation status
internal/tiger/mcp/service_tools.go Updated MCP tool names, descriptions, and parameter naming with enhanced schema documentation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

mcp_feedback.md Outdated
### `mcp__tigerdata__service_create`
**Strengths**: Excellent use of enums, clear defaults, good examples
**✅ IMPLEMENTED**: Renamed `timeout` parameter to `timeout_minutes` for clarity
**Result**: Parameter name now explicitly indicates units, preventing confusion
Copy link

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sentence should end with a period for consistency with other documentation.

Suggested change
**Result**: Parameter name now explicitly indicates units, preventing confusion
**Result**: Parameter name now explicitly indicates units, preventing confusion.

Copilot uses AI. Check for mistakes.
Comment on lines +60 to +61
schema.Properties["service_id"].Description = "The unique identifier of the service (10-character alphanumeric string). Use service_list to find service IDs."
schema.Properties["service_id"].Examples = []any{"e6ue9697jf", "u8me885b93"}
Copy link

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The service_id description and examples are duplicated across multiple schemas. Consider extracting this to a shared constant or helper function to avoid maintenance issues when the description needs to be updated.

Copilot uses AI. Check for mistakes.
Comment on lines +160 to +161
schema.Properties["service_id"].Description = "The unique identifier of the service (10-character alphanumeric string). Use service_list to find service IDs."
schema.Properties["service_id"].Examples = []any{"e6ue9697jf", "u8me885b93"}
Copy link

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The service_id description and examples are duplicated across multiple schemas. Consider extracting this to a shared constant or helper function to avoid maintenance issues when the description needs to be updated.

Copilot uses AI. Check for mistakes.
Comment on lines +214 to +220
Description: "Create a new database service in TigerData Cloud with specified type, compute resources, region, and HA options.\n" +
"\n" +
"Default: Returns immediately (service provisions in background).\n" +
"wait=true: Blocks until service ready.\n" +
"timeout_minutes: Wait duration in minutes (with wait=true).\n" +
"\n" +
"WARNING: Creates billable resources.",
Copy link

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The multi-line string concatenation makes the description harder to read and maintain. Consider using a multi-line string literal or formatted string for better readability.

Suggested change
Description: "Create a new database service in TigerData Cloud with specified type, compute resources, region, and HA options.\n" +
"\n" +
"Default: Returns immediately (service provisions in background).\n" +
"wait=true: Blocks until service ready.\n" +
"timeout_minutes: Wait duration in minutes (with wait=true).\n" +
"\n" +
"WARNING: Creates billable resources.",
Description: `Create a new database service in TigerData Cloud with specified type, compute resources, region, and HA options.
Default: Returns immediately (service provisions in background).
wait=true: Blocks until service ready.
timeout_minutes: Wait duration in minutes (with wait=true).
WARNING: Creates billable resources.`,

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with this feedback.

Copy link
Member

@nathanjcochran nathanjcochran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a handful of questions/comments, but nothing major - feel free to take them or leave them. Overall LGTM ✅ .

Comment on lines +31 to +33
func (ServiceListOutput) Schema() *jsonschema.Schema {
return util.Must(jsonschema.For[ServiceListOutput](nil))
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw, the mcp.AddTool function automatically generates schema for both input types and output types using this exact same logic:

If the tool's input schema is nil, it is set to the schema inferred from the In type parameter. Types are inferred from Go types, and property descriptions are read from the 'jsonschema' struct tag. Internally, the SDK uses the github.com/google/jsonschema-go package for inference and validation. The In type argument must be a map or a struct, so that its inferred JSON Schema has type "object", as required by the spec. As a special case, if the In type is 'any', the tool's input schema is set to an empty object schema value.

If the tool's output schema is nil, and the Out type is not 'any', the output schema is set to the schema inferred from the Out type argument, which must also be a map or struct. If the Out type is 'any', the output schema is omitted.

The only reason I'm doing it manually for the input types (by explicitly setting the InputSchema field rather than leaving it nil) was so that I could add descriptions, examples, enums, etc. If we aren't adding extra metadata for the output types, I don't think we really need this code (unless we just want to match the pattern for the input types for the sake of code clarity, which I'm also fine with).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets match the pattern

Comment on lines +78 to +87
ServiceID string `json:"id" jsonschema:"Service identifier (10-character alphanumeric string)"`
Name string `json:"name"`
Status string `json:"status"`
Type string `json:"type"`
Status string `json:"status" jsonschema:"Service status (e.g., READY, PAUSED, CONFIGURING, UPGRADING)"`
Type string `json:"type" jsonschema:"One of: TIMESCALEDB, POSTGRES, VECTOR"`
Region string `json:"region"`
Created string `json:"created,omitempty"`
Resources *ResourceInfo `json:"resources,omitempty"`
Replicas int `json:"replicas,omitempty"`
DirectEndpoint string `json:"direct_endpoint,omitempty"`
PoolerEndpoint string `json:"pooler_endpoint,omitempty"`
Replicas int `json:"replicas,omitempty" jsonschema:"Number of HA replicas (0=single node/no HA, 1+=HA enabled)"`
DirectEndpoint string `json:"direct_endpoint,omitempty" jsonschema:"Direct database connection endpoint"`
PoolerEndpoint string `json:"pooler_endpoint,omitempty" jsonschema:"Connection pooler endpoint"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we're providing descriptions for some fields, but not others? Is the assumption that some of the fields are self-explanatory and don't need a description? I would kind of expect all of the fields to have a description if any of them do.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only add descriptions to fields that are non-obvious. Fields like name, region, created, and paused are self-explanatory from their names, so adding descriptions would waste tokens without adding value.


This tool provisions a new database service with specified configuration including service type, compute resources, region, and high availability options. By default, the tool returns immediately after the creation request is accepted, but the service may still be provisioning and not ready for connections yet.

Only set 'wait: true' if you need the service to be fully ready immediately after the tool call returns. In most cases, leave wait as false (default) for faster responses.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw, I had previously included this because Claude was almost always setting wait: true, even when not prompted to, and despite the default being set to false. It was just kind of assuming that it would be better to wait for the service to be ready, and didn't seem to fully understand the downsides of pausing the conversation and forcing the user to wait. This message helped it understand that it's often okay to not wait, unless there's a specific reason/need to wait. I didn't try it with your updated descriptions, so it might be fine, but figured I'd mention it since it's something I noticed before.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made the language a bit stronger

Comment on lines +214 to +220
Description: "Create a new database service in TigerData Cloud with specified type, compute resources, region, and HA options.\n" +
"\n" +
"Default: Returns immediately (service provisions in background).\n" +
"wait=true: Blocks until service ready.\n" +
"timeout_minutes: Wait duration in minutes (with wait=true).\n" +
"\n" +
"WARNING: Creates billable resources.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with this feedback.

Comment on lines +91 to +95
- **Input**: `"cpu_memory": "0.5 CPU/2GB"` (combined string)
- **Output**: `{"cpu": "0.5 cores", "memory": "2 GB"}` (separate fields)
- **Impact**: Requires different parsing logic for input vs output
- **Recommendation**: Use consistent format or document the rationale
- **WHY**: Reduces implementation complexity
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, the rationale for the combined input strings is to prevent the LLM from selecting invalid combinations of CPU/memory in the request. Making it a single field with enum values makes it impossible for the LLM to select invalid combinations.

We could do the same for the output, but we don't have the same problem on the output side (and I'm not actually convinced that consistency between input and output really matters to the LLM)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep agreed

cevian and others added 9 commits October 7, 2025 10:45
- Change tiger_service_* to service_* for consistency with CLI commands
- Update tool names: tiger_service_list → service_list, etc.
- Update all comments and cross-references to use new names
- Add comprehensive MCP feedback document with improvement recommendations

Addresses highest priority feedback item: tool naming convention.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add programmatic response schema generation using jsonschema tags
- Enhance service_id parameter documentation with validation patterns
- Clarify wait/timeout behavior in service_create descriptions
- Add selective field descriptions only where they provide value
- Update feedback document with implementation progress

Key improvements:
* Programmatic schemas prevent documentation bitrot
* Service ID validation: ^[a-z0-9]{10}$ with real examples
* Clear replica semantics: 0=single node/no HA, 1+=HA enabled
* Accurate status enum values from OpenAPI spec

Addresses medium priority feedback items: parameter docs and response schemas.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Mark remaining items as SKIPPED with detailed reasoning
- Document resource format inconsistency as known API-level issue
- Complete implementation summary with all decisions explained

Final status: 4/7 items completed, 3/7 skipped for good reasons:
* Error docs would consume context without helping LLM tool selection
* Security docs are already appropriate without unverifiable claims
* Cost estimates would become outdated and create maintenance burden

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Analyze verbosity in TigerData MCP tool descriptions and provide
streamlined alternatives optimized for LLM tool selection. Achieves
~74% reduction in description length while maintaining all essential
information for accurate tool selection.

Key optimizations:
- Remove redundant phrases and "Perfect for" sections
- Front-load critical information about tool functionality
- Keep only operational details that affect usage
- Eliminate use case examples that LLMs can infer

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Mark MCP tool registration issue as resolved. The tools are now
properly registered and accessible with the mcp__tigerdata__ prefix.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Implement concise tool descriptions optimized for LLM tool selection,
achieving ~74% reduction in description length while maintaining all
essential information.

Changes:
- Remove redundant phrases and "Perfect for" sections
- Front-load critical information about tool functionality
- Keep only operational details that affect usage
- Use string concatenation for better code readability
- Delete mcp_reduce.md after implementing suggestions

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Renamed parameter from 'timeout' to 'timeout_minutes' for clarity
- Updates struct field, JSON tag, schema property, and handler logic
- Makes units explicit to prevent confusion (seconds vs minutes)
- Updates tool description to reference new parameter name

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add Schema() methods to ServiceInfo and ServiceDetail with enum constraints for Type field
- Extract service_id schema properties to shared setServiceIDSchemaProperties() helper to reduce duplication
- Convert service_create description to multi-line backtick string for better readability
- Strengthen wait parameter descriptions to discourage unnecessary blocking (wait defaults to false)
- Add period to sentence in mcp_feedback.md for consistency

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@cevian cevian force-pushed the age-160-improve-service-tool-definitions branch from 484c84d to 87f70d6 Compare October 7, 2025 09:27
@cevian cevian merged commit 2fc1a86 into main Oct 7, 2025
1 check passed
@cevian cevian deleted the age-160-improve-service-tool-definitions branch October 7, 2025 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants